feat: adds failure domain api for AWS and EKS #1347
Conversation
| - name: workerConfig | ||
| value: | ||
| aws: | ||
| failureDomain: us-west-2a |
There was a problem hiding this comment.
Were you able to verify this manually for EKS? From the docs it uses failureDomain: "1"
https://cluster-api-aws.sigs.k8s.io/topics/failure-domains/worker-nodes#failure-domains-in-worker-nodes
But in the CAPA code it does look like it should be the AZ like you have it documented
Co-authored-by: Dimitri Koshkin <[email protected]>
| @@ -0,0 +1,73 @@ | |||
| +++ | |||
| title = "AWS Failure Domain" | |||
There was a problem hiding this comment.
| title = "AWS Failure Domain" | |
| title = "EKS Failure Domain" |
| variables: | ||
| - name: workerConfig | ||
| value: | ||
| aws: |
There was a problem hiding this comment.
| aws: | |
| eks: |
| overrides: | ||
| - name: workerConfig | ||
| value: | ||
| aws: |
There was a problem hiding this comment.
| aws: | |
| eks: |
| overrides: | ||
| - name: workerConfig | ||
| value: | ||
| aws: |
There was a problem hiding this comment.
| aws: | |
| eks: |
| overrides: | ||
| - name: workerConfig | ||
| value: | ||
| aws: |
There was a problem hiding this comment.
| aws: | |
| eks: |
| if obj.GetKind() != "MachineDeployment" || obj.GetAPIVersion() != clusterv1.GroupVersion.String() { | ||
| log.V(5).Info("not a MachineDeployment, skipping") | ||
| return nil | ||
| } | ||
|
|
||
| log.WithValues( | ||
| "patchedObjectKind", obj.GetKind(), | ||
| "patchedObjectName", client.ObjectKeyFromObject(obj), | ||
| ).Info("setting failure domain in worker MachineDeployment spec") | ||
|
|
||
| if err := unstructured.SetNestedField( | ||
| obj.Object, | ||
| failureDomainVar, | ||
| "spec", "template", "spec", "failureDomain", | ||
| ); err != nil { | ||
| return err |
There was a problem hiding this comment.
We should be using patches.MutateIfApplicable instead for consistency between all the handlers and use concrete types instead of unstructured
| variableFieldPath []string | ||
| } | ||
|
|
||
| func NewWorkerPatch() *awsFailureDomainWorkerPatchHandler { |
There was a problem hiding this comment.
We should add failure domain patch for AWS control plane too.
supershal
left a comment
supershal
left a comment
There was a problem hiding this comment.
suggested few changes.
yanhua121
left a comment
yanhua121
left a comment
There was a problem hiding this comment.
I think this is similar to the topic that we discussed on where to put the worker nodePool failureDomain reference. For the worker nodePool MachineDeployment, we can take advantage of MD's existing "spec.failureDomain" field (applies to all providers since MD is from CAPI), instead of configuring it via individual provider's cluster.spec.topology.workerConfig and use the injector to MD's spec.failureDomain. We can discuss on this tomorrow if you disagree.
Refer to the thread discussion on where to put the failureDomain reference for worker nodePool for Nutanix CAREN configuration.
**What problem does this PR solve?**:
adds failure domians api to AWS/EKS Machine deployments
Which issue(s) this PR fixes:
https://jira.nutanix.com/browse/NCN-110350
How Has This Been Tested?:
Special notes for your reviewer: